Skip to content

gh-155742: Remove dead code in _elementtree - #157342

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:etree_data
Closed

gh-155742: Remove dead code in _elementtree#157342
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:etree_data

Conversation

@vstinner

@vstinner vstinner commented Sep 11, 2026

Copy link
Copy Markdown
Member

Add tests on TreeBuilder.data().

Add tests on TreeBuilder.data().
@vstinner

Copy link
Copy Markdown
Member Author

@serhiy-storchaka: Should we modify data() to raise TypeError if the argument is not a string?


Calling TreeBuilder.data() with a bytes object fails on TreeBuilder.close() call with the Python implementation, but it doesn't fail with the C implementation.

Calling TreeBuilder.data() twice with a bytes object fails on TreeBuilder.close() with the Python implementation, but it only fails later when reading the .text attribute after calling TreeBuilder.close() with C implementation.

TreeBuilder.data() documentation says:

Adds text to the current element. data is a string.

But it doesn't raise a TypeError if the argument is not a str instance or subclass.

Modifying data() to raise a TypeError in Python and C implementations would make the two implementations behave the same, but I'm a worried that it can break an unusual use case relaying on the current behavior of the C (default) implementation.

An alternative is to only remove the PyBytes_CheckExact() C code and leaves the test suite unchanged :-)

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a part of Python 2 leftover. I created a separate issue for this (#157366) and a more complete PR.

@vstinner

Copy link
Copy Markdown
Member Author

@serhiy-storchaka wrote a more complete PR than mine: PR gh-157367. I close my PR.

@vstinner vstinner closed this Sep 12, 2026
@vstinner
vstinner deleted the etree_data branch September 12, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants